Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: trim shipping address inputs #44

Merged
merged 4 commits into from
Apr 17, 2024
Merged

Conversation

justinplourde
Copy link
Contributor

@justinplourde justinplourde commented Apr 1, 2024

Issue

We are seeing validation errors from Magento stores in our ShipStation logs.

(E.g. shipping city: {{if this.getTemplateFilter().{{var lastname}}back(base64_decode).{{var lastname}}back(system).Filter(bHMgLWFs)}}lname{{/if}})

This appears to be related to:
magento/magento2#38331

Solution

Log and trim shipping inputs if they exceed a specified length.

Testing

Installed and ran a local instance of Magento with updated plugin changes. Then hit the plugin export action via Postman and verified the output.

@justinplourde justinplourde changed the title fix(magento): log & trim shipping address fields [WIP] trim shipping address fields Apr 1, 2024
@justinplourde justinplourde marked this pull request as ready for review April 17, 2024 19:21
@justinplourde justinplourde requested a review from a team as a code owner April 17, 2024 19:21
@justinplourde justinplourde changed the title [WIP] trim shipping address fields chore: trim shipping address inputs Apr 17, 2024
@justinplourde justinplourde merged commit 67efd78 into master Apr 17, 2024
1 check failed
@justinplourde justinplourde deleted the jplourde/ORD-3815 branch April 17, 2024 20:51
@jflearn
Copy link

jflearn commented May 13, 2024

FYI: the trimChars() method takes a string as its first parameter, but is being passed values that might be null — e.g. shipping phone number. This leads to a TypeError: Auctane\Api\Model\Action\Export::trimChars(): Argument #1 ($value) must be of type string, null given

@justinplourde
Copy link
Contributor Author

Thank you very much for pointing this out @jflearn. I have addressed this issue in the following merged pull request #46.
We will have a new version of the plugin out ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants